Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(solid-query): Solid Query Adapter for TanStack Query #4211

Merged
merged 9 commits into from
Sep 24, 2022
Merged

feat(solid-query): Solid Query Adapter for TanStack Query #4211

merged 9 commits into from
Sep 24, 2022

Conversation

lukesmurray
Copy link
Contributor

This pull request aims to add the TanStack Query adapter for Solid JS. This pull request will add the following key primitives to the package.

  • createQuery
  • createQueries
  • createInfiniteQueries
  • createMutation
  • useIsFetching
  • useIsMutating
  • useQueryClient
  • QueryClient
  • QueryClientProvider

The adapter docs have been updated as well as examples have been added.

Acknowledgments

The work on solid-query would not have been possible without

This is a follow-up to #4195 but rebased on main and with commits from beta removed per discussion with @TkDodo and @ardeora.

Copy link
Collaborator

@TkDodo TkDodo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just noting down some improvements to the whole structure that we could also do in a follow-up. Maybe this gets easier with a task runner like turborepo where we also have an open PR

Comment on lines +34 to +43
overrides: [
{
exclude: './packages/solid-query/**',
presets: ['@babel/react'],
},
{
include: './packages/solid-query/**',
presets: ['babel-preset-solid'],
},
],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm no babel expert, but I think it should be possible to also have one babel config per package? So we would only keep the shared things in top level and then move the framework specific things further down ?

@@ -34,5 +34,6 @@ module.exports = {
printBasicPrototype: false,
},
moduleNameMapper,
preset: d.includes("solid") ? 'solid-jest/preset/browser' : undefined,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are currently running jest at the top level for all tests. I think what we should rather do is:

  • have each package define their own jest task that uses its own jest config
  • we can have shared configs in a top level preset

Comment on lines +144 to +158
...buildConfigs({
name: 'solid-query',
packageDir: 'packages/solid-query',
jsName: 'SolidQuery',
outputFile: 'index',
entryFile: 'src/index.ts',
globals: {
'solid-js/store': 'SolidStore',
'solid-js': 'Solid',
'@tanstack/query-core': 'QueryCore',
},
bundleUMDGlobals: [
'@tanstack/query-core',
],
}),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DamianOsipiuk is this fine?

Copy link
Collaborator

@TkDodo TkDodo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 one thing we need to do to make the publish script work is to add solid-query to the package list here (at the end I think):

export const packages: Package[] = [

@lukesmurray
Copy link
Contributor Author

Before this is merged @ardeora has some follow up work that is not included. So please hold off until we have it.

Copy link
Collaborator

@TkDodo TkDodo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's also add one example to .codesandbox/ci.json so that we can see a preview build of solid-query on each PR:

"sandboxes": ["/examples/react/basic", "/examples/react/basic-typescript"],

lukesmurray and others added 2 commits September 23, 2022 11:55
Add an adapter to TanStack Query to support SolidJS.

Co-authored-by: Aryan Deora <adeora@iu.edu>
Co-authored-by: Jen Kaplan <jenniferckaplan@gmail.com>
@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 23, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit de90e6b:

Sandbox Source
@tanstack/query-example-react-basic Configuration
@tanstack/query-example-react-basic-typescript Configuration
@tanstack/query-example-solid-basic-typescript Configuration

ardeora and others added 2 commits September 23, 2022 13:19
Collaborators:

@oscartbeaumont
@lukesmurray
@jennyckaplan

Co-Authored-By: Oscar Beaumont <oscar@otbeaumont.me>
Co-Authored-By: Luke Murray <34020210+lukesmurray@users.noreply.github.com>
Co-Authored-By: Jen Kaplan <25395806+jennyckaplan@users.noreply.github.com>
@ardeora
Copy link
Contributor

ardeora commented Sep 23, 2022

🚨 one thing we need to do to make the publish script work is to add solid-query to the package list here (at the end I think):

export const packages: Package[] = [

Thank you! I added solid-query to the packages in the list below

@ardeora
Copy link
Contributor

ardeora commented Sep 23, 2022

let's also add one example to .codesandbox/ci.json so that we can see a preview build of solid-query on each PR:

"sandboxes": ["/examples/react/basic", "/examples/react/basic-typescript"],

Yep! I added the typescript example and it's working great here https://codesandbox.io/s/tanstack-query-example-solid-basic-typescript-lhsczt

@ardeora
Copy link
Contributor

ardeora commented Sep 23, 2022

Alright I think we should be good to go here. All the tests seem to be passing. And I applied the batching state updates which works great now. Thanks for all the feedback here 😄

@TkDodo
Copy link
Collaborator

TkDodo commented Sep 23, 2022

exciting 🚀

Co-Authored-By: Oscar Beaumont <oscar@otbeaumont.me>
Co-Authored-By: Luke Murray <34020210+lukesmurray@users.noreply.github.com>
Co-Authored-By: Jen Kaplan <25395806+jennyckaplan@users.noreply.github.com>
@aulneau
Copy link

aulneau commented Sep 23, 2022

just wanted to pop in and say great work, this is so exciting!

ardeora and others added 2 commits September 23, 2022 16:10
Co-Authored-By: Oscar Beaumont <oscar@otbeaumont.me>
Co-Authored-By: Luke Murray <34020210+lukesmurray@users.noreply.github.com>
Co-Authored-By: Jen Kaplan <25395806+jennyckaplan@users.noreply.github.com>
Co-Authored-By: Oscar Beaumont <oscar@otbeaumont.me>
Co-Authored-By: Luke Murray <34020210+lukesmurray@users.noreply.github.com>
Co-Authored-By: Jen Kaplan <25395806+jennyckaplan@users.noreply.github.com>
@lukesmurray
Copy link
Contributor Author

Ok everything seems to be ready to go. @TkDodo feel free to merge. Please add @ardeora, @lukesmurray, @jennyckaplan, and @oscartbeaumont as co-authors on the final merge if possible. This was a team effort, led by @ardeora and I would love to see everyone get recognition for their contributions here.

Cheers 🍻

@codecov-commenter
Copy link

codecov-commenter commented Sep 24, 2022

Codecov Report

Base: 96.36% // Head: 96.57% // Increases project coverage by +0.21% 🎉

Coverage data is based on head (de90e6b) compared to base (eab6e2c).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4211      +/-   ##
==========================================
+ Coverage   96.36%   96.57%   +0.21%     
==========================================
  Files          45       72      +27     
  Lines        2281     2949     +668     
  Branches      640      814     +174     
==========================================
+ Hits         2198     2848     +650     
- Misses         80       99      +19     
+ Partials        3        2       -1     
Impacted Files Coverage Δ
src/core/utils.ts
src/devtools/tests/utils.tsx
src/core/mutationObserver.ts
src/react/useQuery.ts
src/react/utils.ts
src/react/Hydrate.tsx
src/react/QueryErrorResetBoundary.tsx
src/devtools/devtools.tsx
src/core/mutation.ts
src/core/mutationCache.ts
... and 107 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@TkDodo
Copy link
Collaborator

TkDodo commented Sep 24, 2022

I was just about to merge this when I took another look at the codesandbox preview build, and it sadly doesn't render anymore:

https://codesandbox.io/s/tanstack-query-example-solid-basic-typescript-17fbsy

The error is:

Uncaught ReferenceError: React is not defined
    at QueryClientProvider (QueryClientProvider.jsx:37:5)
    at dev.js:519:12
    at untrack (dev.js:425:12)
    at Object.fn (dev.js:515:37)
    at runComputation (dev.js:695:22)
    at updateComputation (dev.js:680:3)
    at devComponent (dev.js:526:3)
    at createComponent (dev.js:1228:10)
    at App (index.tsx:142:3)
    at dev.js:519:12

I was working yesterday for sure :/

Co-Authored-By: Oscar Beaumont <oscar@otbeaumont.me>
Co-Authored-By: Luke Murray <34020210+lukesmurray@users.noreply.github.com>
Co-Authored-By: Jen Kaplan <25395806+jennyckaplan@users.noreply.github.com>
@TkDodo TkDodo merged commit fd68b22 into TanStack:main Sep 24, 2022
@OrJDev
Copy link
Contributor

OrJDev commented Sep 24, 2022

I was just about to merge this when I took another look at the codesandbox preview build, and it sadly doesn't render anymore:

https://codesandbox.io/s/tanstack-query-example-solid-basic-typescript-17fbsy

The error is:

Uncaught ReferenceError: React is not defined
    at QueryClientProvider (QueryClientProvider.jsx:37:5)
    at dev.js:519:12
    at untrack (dev.js:425:12)
    at Object.fn (dev.js:515:37)
    at runComputation (dev.js:695:22)
    at updateComputation (dev.js:680:3)
    at devComponent (dev.js:526:3)
    at createComponent (dev.js:1228:10)
    at App (index.tsx:142:3)
    at dev.js:519:12

I was working yesterday for sure :/

same things here,
it does work tho when using @adeora/solid-query instead of @tanstack/solid-query

@TkDodo
Copy link
Collaborator

TkDodo commented Sep 24, 2022

Yes, we are seeing this, too. This seems to be an upstream problem with vite-plugin-solid.

It worked with 2.3.6 but broke with 2.3.7 which was released today.

we actually pinned the plugin version because of this in our official example, which is working:

https://codesandbox.io/s/github/tanstack/query/tree/main/examples/solid/simple?file=/package.json:411-443

@TkDodo
Copy link
Collaborator

TkDodo commented Sep 24, 2022

@ardeora
Copy link
Contributor

ardeora commented Sep 25, 2022

I was just about to merge this when I took another look at the codesandbox preview build, and it sadly doesn't render anymore:
https://codesandbox.io/s/tanstack-query-example-solid-basic-typescript-17fbsy
The error is:

Uncaught ReferenceError: React is not defined
    at QueryClientProvider (QueryClientProvider.jsx:37:5)
    at dev.js:519:12
    at untrack (dev.js:425:12)
    at Object.fn (dev.js:515:37)
    at runComputation (dev.js:695:22)
    at updateComputation (dev.js:680:3)
    at devComponent (dev.js:526:3)
    at createComponent (dev.js:1228:10)
    at App (index.tsx:142:3)
    at dev.js:519:12

I was working yesterday for sure :/

same things here, it does work tho when using @adeora/solid-query instead of @tanstack/solid-query

Yeah this was an issue with vite-plugin-solid that we just recently resolved. Can you upgrade vite-plugin-solid to v2.3.8?

https://stackblitz.com/edit/vitejs-vite-csdjj1?file=package.json,src%2Fmain.tsx,src%2FApp.tsx

Here is a working stackblitz sandbox for reference

@OrJDev
Copy link
Contributor

OrJDev commented Sep 25, 2022

I was just about to merge this when I took another look at the codesandbox preview build, and it sadly doesn't render anymore:
https://codesandbox.io/s/tanstack-query-example-solid-basic-typescript-17fbsy
The error is:

Uncaught ReferenceError: React is not defined
    at QueryClientProvider (QueryClientProvider.jsx:37:5)
    at dev.js:519:12
    at untrack (dev.js:425:12)
    at Object.fn (dev.js:515:37)
    at runComputation (dev.js:695:22)
    at updateComputation (dev.js:680:3)
    at devComponent (dev.js:526:3)
    at createComponent (dev.js:1228:10)
    at App (index.tsx:142:3)
    at dev.js:519:12

I was working yesterday for sure :/

same things here, it does work tho when using @adeora/solid-query instead of @tanstack/solid-query

Yeah this was an issue with vite-plugin-solid that we just recently resolved. Can you upgrade vite-plugin-solid to v2.3.8?

https://stackblitz.com/edit/vitejs-vite-csdjj1?file=package.json,src%2Fmain.tsx,src%2FApp.tsx

Here is a working stackblitz sandbox for reference

Yes it is ok i managed to get it working and added it as an addon to Create JD App,

great work you did there.

@ardeora
Copy link
Contributor

ardeora commented Oct 14, 2022

@all-contributors
please add @lukesmurray for code.

@ardeora
Copy link
Contributor

ardeora commented Oct 14, 2022

@all-contributors
please add @jennyckaplan for code.

@allcontributors
Copy link
Contributor

@ardeora

I've put up a pull request to add @jennyckaplan! 🎉

@ardeora
Copy link
Contributor

ardeora commented Oct 14, 2022

@all-contributors
please add @oscartbeaumont for code.

@allcontributors
Copy link
Contributor

@ardeora

I've put up a pull request to add @oscartbeaumont! 🎉

@vyktoremario
Copy link

Why does this break for every patch version of vite-plugin-solid?
It seems as if 2.3.9 which is the latest version is causing the same error as above.

@lukesmurray
Copy link
Contributor Author

I'm not sure, and I'm sorry you're having that issue. I suggest posting an issue to vite-plugin-solid with some information about the error you're getting. If you're getting an error, it's likely other packages are getting errors as well, and my guess is it is an upstream issue rather than a problem with solid-query. But if you suspect it is an issue with solid-query feel free to open an issue with some more information about the error in this repo, and we will take a look.

@milahu
Copy link

milahu commented Oct 23, 2022

why invent new names for the API?

edit: because react is wrong and solid is right.
solid also has createSignal, createEffect, ...

react-query solid-query
useQuery createQuery
useBaseQuery createBaseQuery
useQueries createQueries
useQueryClient useQueryClient
useHydrate (missing)
useQueryErrorResetBoundary (missing)
useIsFetching useIsFetching
useMutation createMutation
useInfiniteQuery createInfiniteQuery
result state

@milahu
Copy link

milahu commented Oct 23, 2022

@tanstack/solid-query-persist-client draft

because caching + persistence = profit

/*

@tanstack/solid-query-persist-client



usage

this is a drop-in replacement for QueryClientProvider from @tanstack/solid-query

function App() {
  return (
    <PersistQueryClientProvider
        client={queryClient}
        persister={createPersister()}
      >
      <Main/>
    </PersistQueryClientProvider>
  )
}



see also

https://tanstack.com/query/v4/docs/plugins/persistQueryClient#persistqueryclientprovider



based on

QueryClientProvider in
node_modules/@tanstack/solid-query/build/lib/QueryClientProvider.esm.js
node_modules/@tanstack/solid-query/build/solid/QueryClientProvider.jsx

PersistQueryClientProvider in
node_modules/@tanstack/react-query-persist-client/build/lib/PersistQueryClientProvider.esm.js

persistQueryClientSubscribe in
node_modules/@tanstack/query-persist-client-core/build/lib/persist.esm.js

*/



import { createContext, mergeProps, onMount, onCleanup } from 'solid-js';

import { persistQueryClientSubscribe } from "@tanstack/query-persist-client-core"

import { defaultContext } from "@tanstack/solid-query"



// TODO?
// export { persistQueryClient, persistQueryClientRestore, persistQueryClientSave, persistQueryClientSubscribe }
// export { removeOldestQuery }
//export * from '@tanstack/query-persist-client-core';



const QueryClientSharingContext = createContext(false); // If we are given a context, we will use it.
// Otherwise, if contextSharing is on, we share the first and at least one
// instance of the context across the window
// to ensure that if Solid Query is used across
// different bundles or microfrontends they will
// all use the same **instance** of context, regardless
// of module scoping.



function getQueryClientContext(context, contextSharing) {
  if (context) {
    return context;
  }

  if (contextSharing && typeof window !== 'undefined') {
    if (!window.SolidQueryClientContext) {
      window.SolidQueryClientContext = defaultContext;
    }

    return window.SolidQueryClientContext;
  }

  return defaultContext;
}



export const PersistQueryClientProvider = (props) => {
    const mergedProps = mergeProps({
        contextSharing: false,
    }, props);

    let persistQueryClientUnsubscribe

    onMount(() => {
      // same as in non-persisted provider
      //console.log('PersistQueryClientProvider.onMount: mount')
      mergedProps.client.mount()

      // persistence ...

      // note: different API
      // persistQueryClientSubscribe: props.queryClient
      // QueryClientProvider: props.client
      mergedProps.queryClient = mergedProps.client

      // subscribe
      //console.log('PersistQueryClientProvider.onMount: subscribe')
      persistQueryClientUnsubscribe =
      persistQueryClientSubscribe(mergedProps)

      // test: mount later
      //mergedProps.client.mount()
    });

    onCleanup(() => {
      // same as in non-persisted provider
      //console.log('PersistQueryClientProvider.onCleanup: unmount')
      mergedProps.client.unmount()

      // persistence ...

      //console.log('PersistQueryClientProvider.onCleanup: unsubscribe')
      // unsubscribe
      persistQueryClientUnsubscribe()
    })

    const QueryClientContext = getQueryClientContext(mergedProps.context, mergedProps.contextSharing);

    return (
      <QueryClientSharingContext.Provider value={!mergedProps.context && mergedProps.contextSharing}>
        <QueryClientContext.Provider value={mergedProps.client}>
          {mergedProps.children}
        </QueryClientContext.Provider>
      </QueryClientSharingContext.Provider>
    );
};

next challenge:

DOMException: Failed to execute 'put' on 'IDBObjectStore': [object Array] could not be cloned.

localForage/localForage#610

if the object contains a function it can't be stored inside a database

with indexeddb persister from
https://tanstack.com/query/v4/docs/plugins/persistQueryClient#building-a-persister

problem: queryKey is a solidjs proxy object

queryClient.clientState.queries[i].queryKey

https://tanstack.com/query/v4/docs/adapters/solid-query#important-differences-between-solid-query--react-query

// ❌ react version
const queryKey = ["todos", todo]
useQuery(queryKey, fetchTodos)

// ✅ solid version
const queryKey = ["todos", todo()]
const getQueryKey = () => queryKey
createQuery(getQueryKey, fetchTodos)

this looks like a good place to cleanup the queryKey

@tanstack/solid-query/build/solid/createBaseQuery.js

    // NOTE(milahu): this is reactive to options
    createComputed(() => {
        const newDefaultedOptions = queryClient.defaultQueryOptions(options);
        // TODO(milahu): cleanup options?
        observer.setOptions(newDefaultedOptions);
    });

indexeddb persister

related: https://github.com/faassen/solid-dexie

related: withCacheStorage in @solid-primitives/fetch

@lukesmurray
Copy link
Contributor Author

Thank you for the comments @milahu. You figured out the naming logic before I had a chance to reply!

With regard to the missing apis.

  • useQueryErrorResetBoundary was left out consciously because SolidJS has a built-in error boundary while react does not.
  • useHydration is left out because, in the interest of getting solid-query to the community as fast as possible, we released it without server-side rendering support. There are plans to add server-side rendering support, but I'm not sure if anyone is pursuing it at the moment. You can ask in the discord. That's also probably the best place to talk to the maintainers about the current design and propose new features 😄.

About your error, you may want to use unwrap to get the store without a proxy.

About your proposal for the query key. I'm not quite sure if it would work. It depends on what todo is. But if todo is a signal, you're resolving the signal outside of getQueryKey so getQueryKey would not be reactive.

// only runs once
const queryKey = ["todos", todo()]

// runs multiple times but returns the non-reactive array every time
const getQueryKey = () => queryKey

In general, if you use query key factories the fact that query keys are functions can be abstracted away fairly easily and that's probably the approach I would recommend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants